if you define operationId here you can generate a nicer controller function name .e.g

operationId: searchByCrn

Same goes for other endpoint

done

Presumably we know this is a delius user because of the roles this endpoint is limited to?

yes

You'll see this is complaining because you should be using this version instead:

fun getPersonInfoResult(
    crn: String,
    limitedAccessStrategy: LimitedAccessStrategy,
  )

To enable this add something like the following to the bottom of OffenderService (this may not compile, hopefully you get the idea!)

fun Cas2v2UserEntity.limitedAccessStrategy() = when(userType) {
   DELIUS ->  LimitedAccessStrategy.ReturnRestrictedIfLimitedAccess(this.username)
   else -> error("Can't provide strategy for users of type $userType")
}

And then use the extension function on the current user to get the strategy e.g.

deliusOffenderService.getPersonInfoResult(crn, deliusUser.limitedAccessStrategy())

done

i think this is the same as getPersonByNomsNumber(nomsNumber: String, currentUser: NomisUserEntity) but instead of passing a NomisUserEntity (that is deprecated), you instead pass in the actual value from NomisUserEntity that is required (Active Case ID)?

If that's the case, pretty much all of this code is common and can be shared. Could you not delete all of the body of getPersonByNomsNumber(nomsNumber: String, currentUser: NomisUserEntity) and just delegate to your new function e.g.

fun getPersonByNomsNumber(nomsNumber: String, currentUser: NomisUserEntity) = getPersonByNomsNumberAndActiveCaseLoadId(nomsNumber, currentUser.activeNomisCaseloadId)

This means that no more unit tests on the service would be required (as they probably are currently)

done

If you look at the outline for this class there's a top level Cas2v2PeopleSearchGet class containing these 4 inner classes. I think that top level one should be renamed to SearchByNomisId and contain the two existing inner classes.

There should then be another top level inner class called SearchByCrn which contains the two new inner classes

e.g.

Cas2v2PersonSearchTest
   SearchByNomisId
      WhenThereIsAnError
      WhenSuccesfull 
   SearchByCrn
      WhenThereIsAnError
      WhenSuccesfull 

We don't tend to have that third level of inner classes else where in the code base and just have success and error tests in one inner class, i guess this is something CAS2 do!

done